New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix infinite loop on redirect to file #184
Conversation
Hi @mrkane27, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
Sorry - Commented on the issue instead of the PR. Thanks for this cool fix. We would love to take it after adding a failing test in the current code, that gets fixed by this change. |
Thanks for replying! As far as I can tell, the issue only happens on Linux and Mac, using Mono. |
Ah - Cool. We are not yet running tests on mac and linux regularly (but in fact that is what half the team is doing this week, making it all happen for core clr). Lets do this for now
|
[Fact] | ||
public void TestConsoleWindowWidthNotZero() | ||
{ | ||
Assert.NotEqual(0, new NuGet.Common.Console().WindowWidth); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test will only be relevant if someone runs the tests on Linux, with output redirect. I tried messing around with System.Console.SetOut()
and the like at runtime, but Mono does some initialization before that, so it won't apply.
A different take might be to use |
I like this new idea better. Lets update to it or does not sound like an overkill. We are running tests on Linux on a CI regularly starting from this week. |
I thought about this for a bit more, and it might not actually work. For example, if you do I've also run some tests on how |
I think the key is to unblock the hang. In general we are moving NuGet off mono and moving it gradually to coreCLR on unix (see nuget.commandline.xplat package), so coming up with something reasonable (like your suggestion of 80 chars) might be good enough. And then on coreCLR we can try to get things fixed. |
Is this current state what you had in mind? |
Fix NuGet/Home#1893